Optimize delays - realtime, waiting logic and error handling #132
Conversation
WalkthroughAdds a pyinstrument-based Profiler and Agent lifecycle events; integrates the Profiler into Agent startup/shutdown; replaces event-manager polling with asyncio.Event wakeups; converts Gemini plugin streaming to fully async multi-hop; updates examples, deps, tests, and .gitignore. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Agent
participant EventManager
participant Profiler
participant GeminiLLM
User->>Agent: __init__(..., profiler=Profiler(output_path))
Agent->>Profiler: start (pyinstrument)
Agent->>EventManager: register Profiler.on_finish for AgentFinishEvent
Agent->>EventManager: send AgentInitEvent
User->>Agent: run()
Agent->>GeminiLLM: async stream request
loop Multi-hop (≤ MAX_ROUNDS)
GeminiLLM->>GeminiLLM: async stream chunks
GeminiLLM->>GeminiLLM: extract tool calls & index chunks
alt tool calls exist
GeminiLLM->>GeminiLLM: execute tools concurrently
GeminiLLM->>GeminiLLM: sanitize & emit function_response parts
GeminiLLM->>GeminiLLM: send follow-up stream
end
end
GeminiLLM-->>Agent: final response
Agent->>EventManager: send AgentFinishEvent
EventManager->>Profiler: dispatch AgentFinishEvent
Profiler->>Profiler: stop & write profile.html
Profiler-->>User: profile.html
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
yarikdevcom
force-pushed
the
profiling/realtime-data
branch
from
cbf9efc to
69ac841
Compare
yarikdevcom
changed the title
yarikdevcom
force-pushed
the
profiling/realtime-data
branch
from
35eedab to
29b7a8a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (1)
152-158: Apply async/await to follow-up tool call streaming at lines 152 and 158.Line 102 establishes the correct pattern:
await self.chat.send_message_stream()followed byasync for. Line 152 omits theawait, leavingfollow_up_iteras an unawaited coroutine, then line 158 attempts synchronous iteration, which would fail at runtime.- follow_up_iter = self.chat.send_message_stream(parts, config=cfg_with_tools) # type: ignore[arg-type] + follow_up_iter = await self.chat.send_message_stream(parts, config=cfg_with_tools) # type: ignore[arg-type] follow_up_text_parts: List[str] = [] follow_up_last = None next_calls = [] - for idx, chk in enumerate(follow_up_iter): + async for idx, chk in enumerate(follow_up_iter):Note:
enumerate()supports async iteration, so no manual index tracking is needed.agents-core/vision_agents/core/agents/agents.py (1)
544-567: Fix finish() hang when CallEndedEvent already firedSubscribing to
CallEndedEventinsidefinish()means we miss any event that fired earlier. If the remote participant drops before we callawait agent.finish(), the event manager never replays that notification,running_eventstays unset, and this coroutine blocks forever. Please handle the “call already ended” path—e.g., set the event immediately when you detect the connection is no longer active or register the handler earlier so the flag is updated beforefinish()waits.
🧹 Nitpick comments (3)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (1)
117-121: Consider logging ignored extraction errors for debugging.The bare
except: passblocks silence all errors during tool call extraction. While this prevents crashes from malformed chunks, it may hide legitimate issues during development or debugging.Consider adding debug-level logging:
try: chunk_calls = self._extract_tool_calls_from_stream_chunk(chunk) pending_calls.extend(chunk_calls) - except Exception: - pass # Ignore errors in chunk processing + except Exception as e: + logger.debug(f"Ignoring tool call extraction error: {e}")Apply the same pattern at lines 163-168.
Also applies to: 163-168
agents-core/vision_agents/core/agents/events.py (2)
6-11: Well-structured lifecycle event.
AgentInitEventcorrectly inherits fromBaseEvent(rather thanPluginBaseEvent) since this is an agent lifecycle marker, not a plugin-specific operation. Theinit=Falseparameter on the type field appropriately prevents callers from overriding the event type.The docstring could be expanded to clarify the emission context:
- """Event emitted when Agent class initialized.""" + """Event emitted when an Agent instance is initialized. + + This event marks the beginning of an agent's lifecycle and is used + for profiling and tracking agent initialization. + """
13-18: Lifecycle event properly implemented.
AgentFinishEventfollows the same pattern asAgentInitEventand correctly usesBaseEventas the parent class. The structure is clean and consistent.Consider enriching the docstring to provide context about the finish phase:
- """Event emitted when agent.finish() call ended.""" + """Event emitted when an agent's finish() method completes. + + This event marks the end of an agent's lifecycle and is used for + profiling, cleanup tracking, and resource finalization monitoring. + """
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
examples/01_simple_agent_example/uv.lockis excluded by!**/*.lockexamples/other_examples/openai_realtime_webrtc/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
.gitignore(1 hunks)agents-core/vision_agents/core/agents/agents.py(6 hunks)agents-core/vision_agents/core/agents/events.py(1 hunks)agents-core/vision_agents/core/events/manager.py(7 hunks)agents-core/vision_agents/core/profiling/__init__.py(1 hunks)agents-core/vision_agents/core/profiling/base.py(1 hunks)examples/01_simple_agent_example/pyproject.toml(3 hunks)examples/01_simple_agent_example/simple_agent_example.py(2 hunks)examples/other_examples/openai_realtime_webrtc/openai_realtime_example.py(3 hunks)examples/other_examples/openai_realtime_webrtc/pyproject.toml(1 hunks)plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py(5 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Do not modify sys.path in Python code
Docstrings must follow the Google style guide
Files:
examples/01_simple_agent_example/simple_agent_example.pyexamples/other_examples/openai_realtime_webrtc/openai_realtime_example.pyplugins/gemini/vision_agents/plugins/gemini/gemini_llm.pyagents-core/vision_agents/core/agents/events.pyagents-core/vision_agents/core/profiling/__init__.pyagents-core/vision_agents/core/agents/agents.pyagents-core/vision_agents/core/events/manager.pyagents-core/vision_agents/core/profiling/base.py
🧠 Learnings (1)
📚 Learning: 2025-10-13T22:00:34.300Z
Learnt from: dangusev
Repo: GetStream/Vision-Agents PR: 98
File: plugins/deepgram/vision_agents/plugins/deepgram/stt.py:135-150
Timestamp: 2025-10-13T22:00:34.300Z
Learning: In the Deepgram STT plugin (plugins/deepgram/vision_agents/plugins/deepgram/stt.py), the `started()` method is designed to wait for the connection attempt to complete, not to guarantee a successful connection. It's acceptable for the connection attempt to fail, and downstream code handles the case where `self.dg_connection` is `None`. The `_connected_once` event is set in the `finally` block intentionally to signal attempt completion.
Applied to files:
agents-core/vision_agents/core/agents/agents.py
🧬 Code graph analysis (8)
examples/01_simple_agent_example/simple_agent_example.py (2)
agents-core/vision_agents/core/profiling/base.py (1)
Profiler(10-23)plugins/elevenlabs/vision_agents/plugins/elevenlabs/tts.py (1)
TTS(12-74)
examples/other_examples/openai_realtime_webrtc/openai_realtime_example.py (1)
agents-core/vision_agents/core/profiling/base.py (1)
Profiler(10-23)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (1)
agents-core/vision_agents/core/llm/llm_types.py (1)
NormalizedToolCallItem(107-111)
agents-core/vision_agents/core/agents/events.py (1)
agents-core/vision_agents/core/events/base.py (2)
PluginBaseEvent(52-54)BaseEvent(34-48)
agents-core/vision_agents/core/profiling/__init__.py (1)
agents-core/vision_agents/core/profiling/base.py (1)
Profiler(10-23)
agents-core/vision_agents/core/agents/agents.py (3)
agents-core/vision_agents/core/profiling/base.py (1)
Profiler(10-23)agents-core/vision_agents/core/events/manager.py (2)
send(428-472)wait(474-488)agents-core/vision_agents/core/agents/events.py (2)
AgentInitEvent(7-10)AgentFinishEvent(14-17)
agents-core/vision_agents/core/events/manager.py (1)
agents-core/vision_agents/core/events/base.py (1)
ExceptionEvent(97-100)
agents-core/vision_agents/core/profiling/base.py (3)
agents-core/vision_agents/core/events/manager.py (3)
EventManager(56-551)register_events_from_module(219-256)subscribe(301-370)agents-core/vision_agents/core/agents/agents.py (1)
subscribe(285-297)agents-core/vision_agents/core/agents/events.py (1)
AgentFinishEvent(14-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Ruff & mypy
- GitHub Check: unit / Ruff & mypy
- GitHub Check: unit / Test "not integration"
🔇 Additional comments (14)
.gitignore (1)
88-90: Clarify the purpose of opencode.json.The
profile.htmlentry aligns with the profiling infrastructure introduced in this PR. However,opencode.jsonlacks context in the PR description and related changes.Could you clarify what
opencode.jsonis used for and why it's being ignored at the repository root?examples/other_examples/openai_realtime_webrtc/pyproject.toml (1)
13-13: LGTM!The pyinstrument dependency addition aligns with the profiling infrastructure introduced in this PR.
pyproject.toml (1)
83-83: LGTM!Adding pyinstrument as a dev dependency is appropriate for the profiling infrastructure introduced in this PR.
examples/01_simple_agent_example/pyproject.toml (1)
6-6: LGTM!The addition of the vogent plugin and pyinstrument dependency, along with the corresponding source configuration, aligns with the profiling and plugin enhancements in this PR.
Also applies to: 17-17, 23-23, 35-35
agents-core/vision_agents/core/events/manager.py (5)
145-145: LGTM!Introducing
_received_eventfor internal synchronization is a solid improvement over sleep-based polling in event processing.
217-217: LGTM!Sharing
_received_eventduring merge ensures proper synchronization across merged event managers.
472-472: LGTM!Setting
_received_eventafter queueing ensures the processing loop wakes up promptly to handle new events.
528-530: LGTM!Replacing the sleep with
await _received_event.wait()is a more efficient approach that eliminates unnecessary delays while ensuring the loop resumes promptly when events arrive.
536-536: LGTM!Routing exceptions through the standard
send()method ensures they're processed consistently with other events and properly signals the event loop.plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (4)
4-4: LGTM!The migration to
AsyncClientand use of.aioaccessor is consistent with the async pattern adopted in this PR.Also applies to: 40-40, 57-57
102-102: LGTM!The async streaming implementation with
awaitandasync foris correct, and theidxtracking properly annotates emitted events.Also applies to: 110-123
126-137: LGTM!The multi-round tool calling setup with
MAX_ROUNDS = 3, deduplication, and concurrent execution with timeout is well-designed to handle complex tool interactions safely.
138-150: LGTM!The concurrent tool execution with result sanitization and proper conversion to Gemini's function response format is well-implemented.
agents-core/vision_agents/core/agents/events.py (1)
2-2: LGTM! Import updated correctly.The addition of
BaseEventto the imports is necessary for the new agent lifecycle events and follows the existing import pattern.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (1)
152-169: Fix the follow-up stream handling
chat.send_message_streamreturns an awaitable async iterator. Here we neither await it nor iterate withasync for, so we hand a coroutine toenumerate, tripping mypy and blowing up at runtime once tool calls trigger this path. The Google GenAI SDK examples requireasync for chunk in await chat.send_message_stream(...)to consume the stream(googleapis.github.io). Please await the call and switch to an async loop with a manual index, e.g.:- follow_up_iter = self.chat.send_message_stream(parts, config=cfg_with_tools) # type: ignore[arg-type] - - follow_up_text_parts: List[str] = [] - follow_up_last = None - next_calls = [] - - for idx, chk in enumerate(follow_up_iter): + follow_up_iter = await self.chat.send_message_stream(parts, config=cfg_with_tools) # type: ignore[arg-type] + + follow_up_text_parts: List[str] = [] + follow_up_last = None + next_calls = [] + follow_idx = 0 + + async for chk in follow_up_iter: follow_up_last = chk # TODO: unclear if this is correct (item_id and idx) - self._standardize_and_emit_event(chk, follow_up_text_parts, item_id, idx) + self._standardize_and_emit_event(chk, follow_up_text_parts, item_id, follow_idx) + follow_idx += 1agents-core/vision_agents/core/agents/agents.py (1)
544-566: Add timeout and improve error handling in finish().The refactored
finish()method has several concerns:
Missing timeout: Line 560's
await running_event.wait()has no timeout. IfCallEndedEventis never emitted (e.g., due to network issues or edge transport bugs), this will hang indefinitely. Consider adding a timeout parameter or a reasonable default.Subscription cleanup: The
on_endedsubscription (line 553) is never unsubscribed. If the function exits early or is called multiple times, old subscriptions will accumulate.CancelledError handling: Lines 561-563 catch
CancelledError, clear the event, but then continue to line 564 to sendAgentFinishEventand callclose(). Is it intentional to emit the finish event and close even when cancelled? This might be correct, but should be documented.Race condition: Between lines 544 and 553, there's a window where the event could fire before the subscription is registered if
CallEndedEventwas already queued.Consider this refactor:
async def finish(self): """Wait for the call to end gracefully. Subscribes to the edge transport's `call_ended` event and awaits it. If no connection is active, returns immediately. """ if not self._connection: self.logger.info( "🔚 Agent connection is already closed, finishing immediately" ) return - running_event = asyncio.Event() + running_event = asyncio.Event() + subscription = None + with self.span("agent.finish"): # If connection is None or already closed, return immediately if not self._connection: logging.info( "🔚 Agent connection already closed, finishing immediately" ) return @self.edge.events.subscribe async def on_ended(event: CallEndedEvent): running_event.set() self._is_running = False + + subscription = on_ended - # TODO: add members count check (particiapnts left + count = 1 timeout 2 minutes) + # TODO: add members count check (participants left + count = 1 timeout 2 minutes) try: - await running_event.wait() + await asyncio.wait_for(running_event.wait(), timeout=120.0) except asyncio.CancelledError: running_event.clear() + raise # Re-raise to prevent finish event emission when cancelled + except asyncio.TimeoutError: + self.logger.warning("finish() timed out waiting for CallEndedEvent") + finally: + # Clean up subscription + if subscription and hasattr(self.edge.events, 'unsubscribe'): + self.edge.events.unsubscribe(subscription) self.events.send(events.AgentFinishEvent()) await asyncio.shield(self.close())
🧹 Nitpick comments (1)
examples/other_examples/openai_realtime_webrtc/openai_realtime_example.py (1)
14-14: Consider using the Profiler class for consistency.The manual pyinstrument setup duplicates functionality available in
vision_agents.core.profiling.Profiler. For consistency with other examples (e.g.,simple_agent_example.py), consider using the Profiler class which handles start/stop lifecycle automatically viaAgentFinishEvent.If the agent setup supports it, replace manual profiling with:
-import pyinstrument +from vision_agents.core.profiling import Profiler async def start_agent() -> None: - profiler = pyinstrument.Profiler() - profiler.start() # ... agent = Agent( # ... other params + profiler=Profiler(output_path='profiled.html'), ) # ... await agent.finish() - - profiler.stop() - with open('profiled.html', 'w') as f: - f.write(profiler.output_html())Also applies to: 27-28, 78-80
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
examples/01_simple_agent_example/uv.lockis excluded by!**/*.lockexamples/other_examples/openai_realtime_webrtc/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
.gitignore(1 hunks)agents-core/vision_agents/core/agents/agents.py(6 hunks)agents-core/vision_agents/core/agents/events.py(1 hunks)agents-core/vision_agents/core/events/manager.py(7 hunks)agents-core/vision_agents/core/profiling/__init__.py(1 hunks)agents-core/vision_agents/core/profiling/base.py(1 hunks)examples/01_simple_agent_example/pyproject.toml(3 hunks)examples/01_simple_agent_example/simple_agent_example.py(2 hunks)examples/other_examples/openai_realtime_webrtc/openai_realtime_example.py(3 hunks)examples/other_examples/openai_realtime_webrtc/pyproject.toml(1 hunks)plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py(5 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Do not modify sys.path in Python code
Docstrings must follow the Google style guide
Files:
agents-core/vision_agents/core/profiling/__init__.pyagents-core/vision_agents/core/profiling/base.pyexamples/other_examples/openai_realtime_webrtc/openai_realtime_example.pyagents-core/vision_agents/core/agents/events.pyplugins/gemini/vision_agents/plugins/gemini/gemini_llm.pyagents-core/vision_agents/core/agents/agents.pyexamples/01_simple_agent_example/simple_agent_example.pyagents-core/vision_agents/core/events/manager.py
🧠 Learnings (1)
📚 Learning: 2025-10-13T22:00:34.300Z
Learnt from: dangusev
Repo: GetStream/Vision-Agents PR: 98
File: plugins/deepgram/vision_agents/plugins/deepgram/stt.py:135-150
Timestamp: 2025-10-13T22:00:34.300Z
Learning: In the Deepgram STT plugin (plugins/deepgram/vision_agents/plugins/deepgram/stt.py), the `started()` method is designed to wait for the connection attempt to complete, not to guarantee a successful connection. It's acceptable for the connection attempt to fail, and downstream code handles the case where `self.dg_connection` is `None`. The `_connected_once` event is set in the `finally` block intentionally to signal attempt completion.
Applied to files:
agents-core/vision_agents/core/agents/agents.py
🧬 Code graph analysis (8)
agents-core/vision_agents/core/profiling/__init__.py (1)
agents-core/vision_agents/core/profiling/base.py (1)
Profiler(10-23)
agents-core/vision_agents/core/profiling/base.py (3)
agents-core/vision_agents/core/events/manager.py (2)
register_events_from_module(219-256)subscribe(301-370)agents-core/vision_agents/core/agents/agents.py (1)
subscribe(285-297)agents-core/vision_agents/core/agents/events.py (1)
AgentFinishEvent(14-17)
examples/other_examples/openai_realtime_webrtc/openai_realtime_example.py (1)
agents-core/vision_agents/core/profiling/base.py (1)
Profiler(10-23)
agents-core/vision_agents/core/agents/events.py (1)
agents-core/vision_agents/core/events/base.py (2)
PluginBaseEvent(52-54)BaseEvent(34-48)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (1)
agents-core/vision_agents/core/llm/llm_types.py (1)
NormalizedToolCallItem(107-111)
agents-core/vision_agents/core/agents/agents.py (3)
agents-core/vision_agents/core/profiling/base.py (1)
Profiler(10-23)agents-core/vision_agents/core/events/manager.py (2)
send(428-472)wait(474-488)agents-core/vision_agents/core/agents/events.py (2)
AgentInitEvent(7-10)AgentFinishEvent(14-17)
examples/01_simple_agent_example/simple_agent_example.py (2)
agents-core/vision_agents/core/profiling/base.py (1)
Profiler(10-23)plugins/elevenlabs/vision_agents/plugins/elevenlabs/tts.py (1)
TTS(12-74)
agents-core/vision_agents/core/events/manager.py (1)
agents-core/vision_agents/core/events/base.py (1)
ExceptionEvent(97-100)
🪛 GitHub Actions: CI (unit)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py
[error] 158-158: Mypy type-check error: Argument 1 to 'enumerate' has incompatible type 'Coroutine[Any, Any, AsyncIterator[GenerateContentResponse]] | Any'; expected 'Iterable[Any]'.
🔇 Additional comments (15)
.gitignore (1)
88-90: Appropriate additions for profiling and tooling artifacts.Both entries align well with the PR objectives—
profile.htmlcaptures output from the new profiling instrumentation, while/opencode.jsonexcludes root-level code-generation artifacts. The placement under "Artifacts / assets" is sensible, and the use of a leading slash for/opencode.jsonensures proper root-level scoping.examples/other_examples/openai_realtime_webrtc/pyproject.toml (1)
13-13: Dependency addition looks good.The pyinstrument dependency supports the profiling instrumentation added in the example code.
pyproject.toml (1)
83-83: Dev dependency addition is appropriate.Adding pyinstrument to dev dependencies enables profiling across the workspace.
examples/01_simple_agent_example/pyproject.toml (1)
6-6: Dependencies and comment update look good.The changes appropriately add vogent plugin and pyinstrument dependencies, with proper source mappings following the existing pattern.
Also applies to: 17-17, 23-23, 35-35
agents-core/vision_agents/core/events/manager.py (3)
145-145: Event-driven synchronization improves performance.Replacing the polling sleep with
asyncio.Eventprovides better responsiveness and reduces unnecessary wake-ups. The propagation inmergeensures consistency across merged managers.Also applies to: 217-217, 472-472, 528-530
412-412: Logging level adjustments are appropriate.Moving high-frequency event logs from info to debug reduces noise while maintaining observability for debugging.
Also applies to: 545-547
536-536: Revert this review comment — ExceptionEvent is properly handled and will not be dropped.The concern assumes
_prepare_event()could returnNoneforExceptionEvent, but analysis shows this cannot occur.ExceptionEventis registered during initialization (line 147) and has atypeattribute set to"base.exception". When_prepare_event()is called at line 468, it reaches the final validation (line 408) which checks ifevent.type in self._events. SinceExceptionEventis registered, this check passes and the method explicitly returns the event at line 410-411. The event is then successfully appended to the queue at line 470. No code path exists that would silently dropExceptionEvent.Likely an incorrect or invalid review comment.
examples/01_simple_agent_example/simple_agent_example.py (2)
7-7: Profiler integration looks good.The Profiler usage follows the correct pattern, automatically handling lifecycle via
AgentFinishEventsubscription.Also applies to: 27-27
6-6: I need to verify that ElevenLabs is properly imported in the modified file:Confirm TTS provider change from Cartesia to ElevenLabs is intentional.
The codebase shows Cartesia remains actively maintained and used in other examples (e.g.,
plugins/sample_plugin/example/my_example.py, AWS examples,tts_cartesia/example directory), while ElevenLabs is an established, production-ready TTS integration in the framework with support for configurable models and voice IDs. This appears to be a deliberate choice to demonstrate ElevenLabs TTS in this specific example rather than a complete migration. The change is appropriate, and no action is required.agents-core/vision_agents/core/profiling/__init__.py (1)
1-3: Clean public API exposure.The module initialization properly exports the Profiler class, following Python packaging conventions.
agents-core/vision_agents/core/agents/events.py (1)
2-2: Agent lifecycle events are well-defined.The
AgentInitEventandAgentFinishEventclasses follow the established event pattern and provide clear lifecycle hooks for profiling and other observability features.Also applies to: 6-17
agents-core/vision_agents/core/agents/agents.py (4)
54-54: LGTM!The profiler import is clean and follows the project's relative import pattern.
213-216: LGTM!The profiler is correctly integrated into the plugin aggregation loop with proper None-safety checks. The conditional on line 214 ensures that only plugins with event managers are merged.
243-244: Verify event subscribers are ready for AgentInitEvent.The
AgentInitEventis emitted at the end of__init__, but event handler subscriptions are registered later in thejoin()method (lines 478-479). This means:
- The profiler's
on_finishhandler is subscribed in the Profiler's__init__(line 17 of base.py), so it should receive events- Other handlers registered in
_setup_llm_events()and_setup_speech_events()won't be active yetEnsure that all intended subscribers to
AgentInitEventare registered before this event is sent, or document that this event is only for early-stage subscribers like the profiler.
158-158: Clarify whether log_level default was actually changed from INFO to DEBUG.The review comment asserts that "the default log level has been changed from INFO to DEBUG," but I found no evidence supporting this claimed transition:
- No git history for the
log_levelparameter- No test files document prior default values
- No documentation or README shows prior configuration
- The Agent class docstring example does not mention
log_levelusageThe parameter currently defaults to
logging.DEBUGat line 158, andconfigure_default_logging()applies it to handlers. However, without verifiable evidence of a prior INFO default, the factual claim of a "change" cannot be confirmed.The underlying concern about DEBUG-level logging producing excessive verbosity is worth addressing, but verify first whether this is a newly added parameter or an existing one that was always DEBUG.
Likely an incorrect or invalid review comment.
yarikdevcom
force-pushed
the
profiling/realtime-data
branch
from
16bc202 to
b87b512
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (1)
152-169: Fix async follow-up streaming loop
send_message_streamon the async Gemini client must be awaited, and the resulting stream must be consumed withasync for. Here we neither await the call nor use async iteration, so the first tool round will blow up withTypeError: 'async_generator' object is not iterable(and no follow-up chunks will be processed). The official samples showasync for chunk in await chat.send_message_stream(...), which we should mirror.(pypi.org)
Please apply something like:- follow_up_iter = self.chat.send_message_stream(parts, config=cfg_with_tools) # type: ignore[arg-type] + follow_up_iter = await self.chat.send_message_stream(parts, config=cfg_with_tools) # type: ignore[arg-type] @@ - for idx, chk in enumerate(follow_up_iter): + follow_up_idx = 0 + async for chk in follow_up_iter: follow_up_last = chk # TODO: unclear if this is correct (item_id and idx) - self._standardize_and_emit_event(chk, follow_up_text_parts, item_id, idx) + self._standardize_and_emit_event(chk, follow_up_text_parts, item_id, follow_up_idx) @@ - current_calls = next_calls + follow_up_idx += 1 + current_calls = next_calls
🧹 Nitpick comments (3)
examples/other_examples/openai_realtime_webrtc/openai_realtime_example.py (3)
14-14: Consider using the framework's Profiler abstraction.The example imports
pyinstrumentdirectly, but the codebase now providesvision_agents.core.profiling.Profilerwhich wraps pyinstrument and integrates with the agent lifecycle via events.Import the framework's Profiler instead:
-import pyinstrument +from vision_agents.core.profiling import Profiler
27-28: Refactor to use the framework's Profiler class.The manual profiler instantiation and start call can be replaced with the framework's
Profilerclass, which auto-starts profiling and integrates with the agent lifecycle.Apply this diff:
- profiler = pyinstrument.Profiler() - profiler.start() + profiler = Profiler(output_path='profiled.html')Then pass the profiler to the Agent constructor at line 37:
agent = Agent( edge=getstream.Edge(), agent_user=agent_user, instructions=(...), llm=openai.Realtime(), processors=[], profiler=profiler, # Add this parameter )
78-81: Remove manual profiler cleanup if using the framework's Profiler.If you refactor to use
vision_agents.core.profiling.Profiler(as suggested earlier), these lines become redundant. The Profiler class automatically stops profiling and writes HTML output whenAgentFinishEventis emitted byagent.finish().Remove these lines after switching to the framework's Profiler:
- profiler.stop() - with open('profiled.html', 'w') as f: - f.write(profiler.output_html()) -
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
examples/01_simple_agent_example/uv.lockis excluded by!**/*.lockexamples/other_examples/openai_realtime_webrtc/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
.gitignore(1 hunks)agents-core/vision_agents/core/agents/agents.py(6 hunks)agents-core/vision_agents/core/agents/events.py(1 hunks)agents-core/vision_agents/core/events/manager.py(7 hunks)agents-core/vision_agents/core/profiling/__init__.py(1 hunks)agents-core/vision_agents/core/profiling/base.py(1 hunks)examples/01_simple_agent_example/pyproject.toml(3 hunks)examples/01_simple_agent_example/simple_agent_example.py(2 hunks)examples/other_examples/openai_realtime_webrtc/openai_realtime_example.py(3 hunks)examples/other_examples/openai_realtime_webrtc/pyproject.toml(1 hunks)plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py(5 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- .gitignore
- examples/01_simple_agent_example/pyproject.toml
- pyproject.toml
- agents-core/vision_agents/core/agents/events.py
- agents-core/vision_agents/core/profiling/base.py
- examples/01_simple_agent_example/simple_agent_example.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Do not modify sys.path in Python code
Docstrings must follow the Google style guide
Files:
agents-core/vision_agents/core/profiling/__init__.pyagents-core/vision_agents/core/agents/agents.pyagents-core/vision_agents/core/events/manager.pyplugins/gemini/vision_agents/plugins/gemini/gemini_llm.pyexamples/other_examples/openai_realtime_webrtc/openai_realtime_example.py
🧠 Learnings (1)
📚 Learning: 2025-10-13T22:00:34.300Z
Learnt from: dangusev
Repo: GetStream/Vision-Agents PR: 98
File: plugins/deepgram/vision_agents/plugins/deepgram/stt.py:135-150
Timestamp: 2025-10-13T22:00:34.300Z
Learning: In the Deepgram STT plugin (plugins/deepgram/vision_agents/plugins/deepgram/stt.py), the `started()` method is designed to wait for the connection attempt to complete, not to guarantee a successful connection. It's acceptable for the connection attempt to fail, and downstream code handles the case where `self.dg_connection` is `None`. The `_connected_once` event is set in the `finally` block intentionally to signal attempt completion.
Applied to files:
agents-core/vision_agents/core/agents/agents.py
🧬 Code graph analysis (5)
agents-core/vision_agents/core/profiling/__init__.py (1)
agents-core/vision_agents/core/profiling/base.py (1)
Profiler(10-23)
agents-core/vision_agents/core/agents/agents.py (3)
agents-core/vision_agents/core/profiling/base.py (1)
Profiler(10-23)agents-core/vision_agents/core/events/manager.py (2)
send(428-472)wait(474-488)agents-core/vision_agents/core/agents/events.py (2)
AgentInitEvent(7-10)AgentFinishEvent(14-17)
agents-core/vision_agents/core/events/manager.py (1)
agents-core/vision_agents/core/events/base.py (1)
ExceptionEvent(97-100)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (1)
agents-core/vision_agents/core/llm/llm_types.py (1)
NormalizedToolCallItem(107-111)
examples/other_examples/openai_realtime_webrtc/openai_realtime_example.py (1)
agents-core/vision_agents/core/profiling/base.py (1)
Profiler(10-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: unit / Ruff & mypy
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Ruff & mypy
🔇 Additional comments (12)
examples/other_examples/openai_realtime_webrtc/pyproject.toml (1)
13-13: Good addition for profiling support in examples.The pyinstrument>=5.1.1 dependency exists and was released Aug 12, 2025, providing the profiling tooling needed for the agent event profiling hooks mentioned in the PR objectives. The version constraint is appropriately permissive for an example project.
agents-core/vision_agents/core/profiling/__init__.py (1)
1-3: LGTM!Standard module initialization that correctly exposes the
Profilerclass as the public API.agents-core/vision_agents/core/agents/agents.py (4)
54-54: LGTM!The import correctly brings in the Profiler class for integration into the agent lifecycle.
159-159: LGTM!The profiler parameter and plugin integration correctly wire the profiler into the agent's event system. The
hasattrcheck at line 214 safely handlesNoneprofiler instances, and merging profiler events enables lifecycle-based profiling viaAgentFinishEvent.Also applies to: 213-216
243-244: LGTM!Emitting
AgentInitEventat the end of initialization correctly signals that the agent setup is complete, enabling subscribers to react to the agent lifecycle.
544-566: Review cancellation handling and subscription cleanup.The refactored
finish()method improves upon busy-waiting, but consider these concerns:
Cancellation behavior: Lines 561-562 catch
CancelledErrorand clear the event, but execution continues to sendAgentFinishEventand callclose(). Iffinish()is cancelled, should it propagate the cancellation rather than proceeding with cleanup?Subscription leak: The
on_endedhandler at line 553 subscribes toedge.eventsbut never unsubscribes. Iffinish()is called multiple times (though discouraged by the class docstring), multiple subscriptions accumulate.Consider these adjustments:
Option 1: Propagate cancellation
try: await running_event.wait() except asyncio.CancelledError: - running_event.clear() + self.logger.debug("finish() was cancelled") + raiseOption 2: Unsubscribe on completion
@self.edge.events.subscribe async def on_ended(event: CallEndedEvent): running_event.set() self._is_running = False try: await running_event.wait() except asyncio.CancelledError: running_event.clear() finally: self.edge.events.unsubscribe(on_ended) # Clean up subscriptionVerify the intended behavior when
finish()is cancelled.agents-core/vision_agents/core/events/manager.py (6)
145-145: LGTM!The
_received_eventfield correctly initializes anasyncio.Eventfor coordinating producer-consumer wakeups.
217-217: LGTM!Propagating
_received_eventduring merge ensures that both managers coordinate on the same synchronization primitive, maintaining correct producer-consumer behavior.
412-412: LGTM!Reducing event reception logging to DEBUG level appropriately decreases log verbosity for this high-frequency operation.
472-473: LGTM!Signaling
_received_eventafter enqueuing events correctly wakes the consumer to process the newly queued work. Signaling once after all events are enqueued is efficient.
528-530: LGTM!The wait-and-clear pattern correctly implements event-based consumer wakeup, replacing inefficient sleep polling. The pattern is race-free given the single-consumer design and atomic deque operations.
536-540: LGTM!Using
self.send()forExceptionEventensures proper event preparation and signaling, maintaining consistency with the producer-consumer pattern. The additional debug logging with module names improves observability.Also applies to: 545-547
…log level default in Agent class
…ion and event handling
yarikdevcom
force-pushed
the
profiling/realtime-data
branch
from
592faf4 to
58d5da4
Compare
… pyproject.toml and uv.lock
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (1)
92-100: Await the async chat creationLine 95 calls
self.client.chats.create(...)but never awaits it. WithAsyncClient, that method returns a coroutine, soself.chatbecomes a coroutine object and the laterself.chat.send_message_stream(...)access fails at runtime ('coroutine' object has no attribute 'send_message_stream'). Please await the creation before storing the chat.- self.chat = self.client.chats.create(model=self.model, config=config) + self.chat = await self.client.chats.create(model=self.model, config=config)
🧹 Nitpick comments (1)
examples/01_simple_agent_example/simple_agent_example.py (1)
27-27: Consider making profiler optional for examples.The profiler is always instantiated in this example. For demonstration purposes, consider making it conditional or documenting that users should remove it when not needed:
# Enable profiling (remove in production) profiler=Profiler() if os.getenv("ENABLE_PROFILING") else None,This would better demonstrate best practices for optional profiling in production code.
Otherwise, the profiler integration follows the documented pattern correctly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
examples/01_simple_agent_example/uv.lockis excluded by!**/*.lockexamples/other_examples/openai_realtime_webrtc/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
.gitignore(1 hunks)DEVELOPMENT.md(1 hunks)agents-core/vision_agents/core/agents/agents.py(6 hunks)agents-core/vision_agents/core/agents/events.py(1 hunks)agents-core/vision_agents/core/events/manager.py(7 hunks)agents-core/vision_agents/core/profiling/__init__.py(1 hunks)agents-core/vision_agents/core/profiling/base.py(1 hunks)examples/01_simple_agent_example/pyproject.toml(3 hunks)examples/01_simple_agent_example/simple_agent_example.py(2 hunks)examples/other_examples/openai_realtime_webrtc/openai_realtime_example.py(0 hunks)examples/other_examples/openai_realtime_webrtc/pyproject.toml(1 hunks)plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py(6 hunks)pyproject.toml(1 hunks)
💤 Files with no reviewable changes (1)
- examples/other_examples/openai_realtime_webrtc/openai_realtime_example.py
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (6)
- agents-core/vision_agents/core/events/manager.py
- agents-core/vision_agents/core/agents/agents.py
- agents-core/vision_agents/core/profiling/init.py
- examples/other_examples/openai_realtime_webrtc/pyproject.toml
- agents-core/vision_agents/core/agents/events.py
- agents-core/vision_agents/core/profiling/base.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Do not modify sys.path in Python code
Docstrings must follow the Google style guide
Files:
examples/01_simple_agent_example/simple_agent_example.pyplugins/gemini/vision_agents/plugins/gemini/gemini_llm.py
🧠 Learnings (1)
📚 Learning: 2025-11-05T10:57:48.635Z
Learnt from: yarikdevcom
Repo: GetStream/Vision-Agents PR: 132
File: agents-core/vision_agents/core/profiling/base.py:19-23
Timestamp: 2025-11-05T10:57:48.635Z
Learning: In the Profiler class in agents-core/vision_agents/core/profiling/base.py, synchronous file I/O is acceptable for writing the profile HTML output in the on_finish method since it executes at agent shutdown and blocking is not a concern.
Applied to files:
DEVELOPMENT.md
🧬 Code graph analysis (2)
examples/01_simple_agent_example/simple_agent_example.py (2)
agents-core/vision_agents/core/profiling/base.py (1)
Profiler(10-48)plugins/elevenlabs/vision_agents/plugins/elevenlabs/tts.py (1)
TTS(12-74)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (3)
agents-core/vision_agents/core/llm/llm_types.py (1)
NormalizedToolCallItem(107-111)plugins/openai/vision_agents/plugins/openai/openai_llm.py (1)
_standardize_and_emit_event(466-518)agents-core/vision_agents/core/llm/llm.py (1)
_extract_tool_calls_from_stream_chunk(152-166)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: unit / Ruff & mypy
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Ruff & mypy
🔇 Additional comments (7)
pyproject.toml (1)
89-89: LGTM! Appropriate dev dependency for profiling.The addition of
pyinstrument>=5.1.1correctly supports the newProfilerclass introduced in this PR.examples/01_simple_agent_example/simple_agent_example.py (2)
6-7: LGTM! Appropriate imports for profiling functionality.The imports correctly bring in both the
elevenlabsplugin and the newProfilerclass.
24-24: TTS provider switch seems unrelated to profiling optimization.The change from
cartesia.TTS()toelevenlabs.TTS()doesn't appear connected to the PR's stated objective of "Optimize delays - realtime, waiting logic and error handling." Was this an intentional change for this PR, or should it be in a separate commit?examples/01_simple_agent_example/pyproject.toml (3)
6-6: LGTM! Improved comment clarity.The updated comment "Dependencies that the example actually uses" is clearer than the previous version.
17-17: LGTM! Fixes missing vogent dependency.The
vogentplugin was already being used insimple_agent_example.py(line 26:vogent.TurnDetection()), so this change correctly declares the missing dependency.Also applies to: 35-35
23-23: LGTM! Supports profiling functionality.The
pyinstrument>=5.1.1dependency correctly supports theProfilerusage added to the example.DEVELOPMENT.md (1)
304-340: No duplicates found; documentation verified.The script output confirms only one occurrence of the profiling section in DEVELOPMENT.md:
- "### Profiling" header appears once at line 304
- Profiler class description appears once at line 306
The AI summary's claim of a duplicate identical section is incorrect. The profiling documentation requires no changes.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (1)
43-60: Fix docstring: "Anthropic client" should be "Google client".The docstring on line 50 incorrectly references "Anthropic client" instead of "Google client" or "Gemini client". This appears to be a copy-paste artifact from another LLM plugin.
Apply this diff to correct the docstring:
Args: model (str): The model to use. api_key: optional API key. by default loads from GOOGLE_API_KEY - client: optional Anthropic client. by default creates a new client object. + client: optional Google Gemini async client. by default creates a new client object.
♻️ Duplicate comments (1)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (1)
178-182: Same silent exception pattern as lines 124-128.This is the same silent exception handling pattern flagged earlier. Consider adding debug logging here as well to aid troubleshooting.
🧹 Nitpick comments (3)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (3)
110-110: Style: Extra space before colon.Minor formatting inconsistency:
text_parts : List[str]has an extra space before the colon. Python style typically omits this space.Apply this diff:
- text_parts : List[str] = [] + text_parts: List[str] = []
124-128: Consider logging silenced tool call extraction errors.The bare
passstatement silently suppresses all exceptions during tool call extraction from chunks. While this prevents streaming interruption, it may hide legitimate bugs in_extract_tool_calls_from_stream_chunk.Consider logging at debug level:
try: chunk_calls = self._extract_tool_calls_from_stream_chunk(chunk) pending_calls.extend(chunk_calls) - except Exception: - pass # Ignore errors in chunk processing + except Exception as e: + # Log but don't interrupt streaming + import logging + logging.debug(f"Failed to extract tool calls from chunk: {e}")
135-135: Consider making MAX_ROUNDS configurable.The hardcoded
MAX_ROUNDS = 3limit for multi-hop tool execution could be a configuration parameter, allowing users to adjust the depth based on their use case.Example approach:
+ def __init__(self, model: str, api_key: Optional[str] = None, client: Optional[AsyncClient] = None, max_tool_rounds: int = 3): """ Initialize the GeminiLLM class. Args: model (str): The model to use. api_key: optional API key. by default loads from GOOGLE_API_KEY client: optional Google Gemini async client. by default creates a new client object. + max_tool_rounds: maximum number of tool calling rounds. Default is 3. """ super().__init__() self.events.register_events_from_module(events) self.model = model + self.max_tool_rounds = max_tool_rounds self.chat: Optional[Any] = NoneThen use
self.max_tool_roundsinstead of the localMAX_ROUNDSvariable.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
examples/other_examples/openai_realtime_webrtc/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Do not modify sys.path in Python code
Docstrings must follow the Google style guide
Files:
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: yarikdevcom
Repo: GetStream/Vision-Agents PR: 132
File: agents-core/vision_agents/core/profiling/base.py:19-23
Timestamp: 2025-11-05T10:57:48.635Z
Learning: In the Profiler class in agents-core/vision_agents/core/profiling/base.py, synchronous file I/O is acceptable for writing the profile HTML output in the on_finish method since it executes at agent shutdown and blocking is not a concern.
🧬 Code graph analysis (1)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (3)
agents-core/vision_agents/core/llm/llm_types.py (1)
NormalizedToolCallItem(107-111)plugins/openai/vision_agents/plugins/openai/openai_llm.py (1)
_standardize_and_emit_event(466-518)agents-core/vision_agents/core/llm/llm.py (1)
_extract_tool_calls_from_stream_chunk(152-166)
🔇 Additional comments (4)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (4)
2-4: Async imports look good.The addition of
AsyncIteratorandAsyncClientaligns with the new async streaming implementation.
189-205: Text aggregation and event completion logic looks solid.The logic correctly prioritizes follow-up text when tool calls are executed, and falls back to the initial response text otherwise. The final
LLMResponseCompletedEventemission properly captures the terminal state.
225-374: Helper methods are well-implemented.The helper methods demonstrate good defensive programming:
_standardize_and_emit_eventproperly forwards both native and standardized events_convert_tools_to_provider_formatcorrectly transforms tool schemas for Gemini- Tool call extraction methods use
getattrwith defaults and exception handling_create_tool_result_partshas a sensible fallback for serialization errors
165-185: Now let me check the actual Gemini plugin implementation to understand better how item_id and idx are being used:Let me search for the specific buffer implementation and how Gemini plugin handles follow-up responses:Address follow-up response event indexing to prevent duplicate content_index values.
The review identifies a legitimate issue:
follow_up_idxresets to 0 for each follow-up round whileitem_idremains constant. This creates duplicate(item_id, content_index)pairs when streaming chunks. The conversation layer'sadd_fragment(content_index, content)method expects monotonically increasing indices per item_id to correctly reassemble out-of-order fragments.Two solutions:
- Generate a new
item_idfor each follow-up response (treating follow-ups as separate messages)- Continue incrementing
idxinstead of resetting it (treating the entire exchange as one message with multiple rounds)Choose based on how follow-up responses should be semantically represented in the conversation history.
…cture and async iterator
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/test_function_calling.py (1)
322-336: Consider adding an integration test to validate mock structure.The mock response structure follows the expected pattern for Gemini's function calling, but without exercising the actual Client API, there's risk of drift between the mock and real API. Since coding guidelines prohibit mocking, consider marking this as an integration test that uses the real API.
Based on coding guidelines: "Never use mocking utilities (e.g., unittest.mock, pytest-mock) in test files" and "Mark integration tests with @pytest.mark.integration", this test should either:
- Be converted to an integration test using the real Gemini API, or
- Be removed if unit testing without mocks isn't feasible
This aligns with the project's testing guidelines. As per coding guidelines.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/test_function_calling.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
tests/**/*.py: Never use mocking utilities (e.g., unittest.mock, pytest-mock) in test files
Write tests using pytest (avoid unittest.TestCase or other frameworks)
Mark integration tests with @pytest.mark.integration
Do not use @pytest.mark.asyncio; async support is automatic
Files:
tests/test_function_calling.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Do not modify sys.path in Python code
Docstrings must follow the Google style guide
Files:
tests/test_function_calling.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Ruff & mypy
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Ruff & mypy
🔇 Additional comments (2)
tests/test_function_calling.py (2)
337-337: LGTM!Setting
textto empty for function-call-only responses aligns with realistic LLM behavior when tool calls are returned without accompanying narrative text.
362-376: Same verification concern as the function calling test.This test has the same elaborate mock setup but may not exercise the async client if
call_functionoperates directly on the registry.See the verification script in the previous comment (lines 306-320) to confirm whether the GeminiLLM implementation actually uses the mocked client during these test calls.
…ide instructions for enabling it
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
examples/01_simple_agent_example/simple_agent_example.py (2)
12-12: Add a docstring following Google style guide.The function lacks a docstring. As per coding guidelines, all Python functions should include Google-style docstrings to document purpose, parameters, and return values.
As per coding guidelines.
64-64: Add a docstring following Google style guide.The function lacks a docstring explaining its purpose and telemetry setup details.
As per coding guidelines.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
examples/01_simple_agent_example/simple_agent_example.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Do not modify sys.path in Python code
Docstrings must follow the Google style guide
Files:
examples/01_simple_agent_example/simple_agent_example.py
🧬 Code graph analysis (1)
examples/01_simple_agent_example/simple_agent_example.py (3)
plugins/elevenlabs/tests/test_tts.py (1)
tts(12-18)plugins/cartesia/vision_agents/plugins/cartesia/tts.py (1)
TTS(20-94)plugins/elevenlabs/vision_agents/plugins/elevenlabs/tts.py (1)
TTS(12-74)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Ruff & mypy
- GitHub Check: unit / Ruff & mypy
- GitHub Check: unit / Test "not integration"
🔇 Additional comments (2)
examples/01_simple_agent_example/simple_agent_example.py (2)
6-7: LGTM: Import changes align with TTS backend swap.The switch from cartesia to elevenlabs is consistent with the TTS initialization change below, and the commented profiler import correctly addresses the past review feedback.
27-29: Profiler comment addresses past review feedback.The profiler is correctly disabled by default as requested in past review. The commented-out line provides clear guidance for users who want to enable profiling.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (2)
43-60: Fix docstring and clarify type inconsistency.Two issues here:
- Line 50: The docstring mentions "Anthropic client" but this is the Gemini plugin.
- Type mismatch: The constructor parameter is typed as
Optional[AsyncClient], but whenclientisNone, the code createsClient(api_key=api_key).aio. Verify that.aioreturns anAsyncClientinstance. If it returns a different type, update the type hint accordingly.Apply this diff to fix the docstring:
Args: model (str): The model to use. api_key: optional API key. by default loads from GOOGLE_API_KEY - client: optional Anthropic client. by default creates a new client object. + client: optional Gemini async client. by default creates a new client object.
132-188: Extract hardcoded constant and improve error handling.The verification confirms three actionable refactoring needs:
Line 135:
MAX_ROUNDS = 3should be extracted as a class constant for visibility and configurability.Lines 178-182: The bare
except Exception: passsilently suppresses errors during tool call extraction. Add logging to surface failures.Lines 172-184: The async iteration loop over
follow_up_iterlacks exception handling. If the stream fails during iteration, the exception propagates uncaught. Wrap the loop in try-except to handle stream failures gracefully.Apply this diff to extract the constant at the class level:
+ MAX_TOOL_ROUNDS = 3 + async def send_message(self, *args, **kwargs): """ send_message gives you full support/access to the native Gemini chat send message method under the hood it calls chat.send_message_stream(*args, **kwargs) this method wraps and ensures we broadcast an event which the agent class hooks into """Then update line 135:
if pending_calls: # Multi-hop tool calling loop - MAX_ROUNDS = 3 rounds = 0 current_calls = pending_calls cfg_with_tools = kwargs.get("config") seen: set[str] = set() - while current_calls and rounds < MAX_ROUNDS: + while current_calls and rounds < self.MAX_TOOL_ROUNDS:
🧹 Nitpick comments (1)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (1)
108-130: Improve exception handling in tool call extraction.The
awaitpattern on line 109 is correct — the async client (client.aio) requiresawaitonsend_message_stream()to return an AsyncIterator.However, lines 124–128 still warrant attention:
- Lines 124–128: The bare
except Exception: passsilently suppresses all errors during tool call extraction. Instead, either log the exception for debugging purposes or catch only specific exception types that are expected and safe to ignore.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Do not modify sys.path in Python code
Docstrings must follow the Google style guide
Files:
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py
🧬 Code graph analysis (1)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (3)
agents-core/vision_agents/core/llm/llm_types.py (1)
NormalizedToolCallItem(107-111)plugins/openai/vision_agents/plugins/openai/openai_llm.py (1)
_standardize_and_emit_event(466-518)agents-core/vision_agents/core/llm/llm.py (1)
_extract_tool_calls_from_stream_chunk(152-166)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Ruff & mypy
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Ruff & mypy
🔇 Additional comments (2)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (2)
189-205: LGTM! Response assembly logic is sound.The final response assembly correctly prioritizes follow-up text over initial text when tool calls were executed, and properly emits the standardized completion event.
225-252: LGTM! Event emission pattern is consistent.The standardization and emission logic properly forwards both native and standardized events, consistent with the OpenAI plugin pattern shown in the relevant code snippets.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (3)
56-56: Fix incorrect docstring reference.The docstring incorrectly references "Anthropic client" instead of "Gemini client."
Apply this diff:
- client: optional Anthropic client. by default creates a new client object. + client: optional Gemini client. by default creates a new client object.
132-136: Avoid silently swallowing exceptions.The bare
exceptwithpasswill hide all errors during tool call extraction, making debugging difficult. Consider logging the exception or at least limiting the scope to expected exceptions.Apply this diff to add logging:
# collect function calls as they stream try: chunk_calls = self._extract_tool_calls_from_stream_chunk(chunk) pending_calls.extend(chunk_calls) - except Exception: - pass # Ignore errors in chunk processing + except Exception as e: + # Log but don't fail on extraction errors + import logging + logging.debug(f"Failed to extract tool calls from chunk: {e}")
190-194: Avoid silently swallowing exceptions (duplicate pattern).Similar to the earlier comment, this bare
exceptwithpasswill hide errors during follow-up tool call extraction.Apply this diff to add logging:
# Check for new function calls try: chunk_calls = self._extract_tool_calls_from_stream_chunk(chk) next_calls.extend(chunk_calls) - except Exception: - pass + except Exception as e: + # Log but don't fail on extraction errors + import logging + logging.debug(f"Failed to extract tool calls from follow-up chunk: {e}")
🧹 Nitpick comments (3)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (3)
143-143: Move MAX_ROUNDS to a class constant.Hardcoding
MAX_ROUNDS = 3inside the method makes it harder to configure. Consider defining it as a class-level constant for better maintainability.Add at the class level (after line 24):
class GeminiLLM(LLM): """...""" MAX_TOOL_ROUNDS = 3Then reference it as
self.MAX_TOOL_ROUNDSin the method.
355-386: Remove or use the unused helper method.The
_create_tool_result_partsmethod is defined but never called. The functionality is implemented inline at lines 167-171. Either refactor to use this method or remove it to avoid dead code.Option 1 - Remove the method if not needed:
- def _create_tool_result_parts( - self, tool_calls: List[NormalizedToolCallItem], results: List[Any] - ): - """...""" - # entire method bodyOption 2 - Refactor lines 167-171 to use this method instead of inline logic.
331-332: Consider logging errors in extraction methods.Both
_extract_tool_calls_from_responseand_extract_tool_calls_from_stream_chunkuse bareexcept: passwhich will hide errors. While these are helper methods that gracefully degrade, consider adding debug logging to aid troubleshooting.Also applies to: 352-353
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python.mdc)
**/*.py: Do not modify sys.path in Python code
Docstrings must follow the Google style guide
Files:
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py
🧬 Code graph analysis (1)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (3)
plugins/aws/vision_agents/plugins/aws/aws_llm.py (1)
client(84-91)agents-core/vision_agents/core/llm/llm_types.py (1)
NormalizedToolCallItem(107-111)plugins/openai/vision_agents/plugins/openai/openai_llm.py (1)
_standardize_and_emit_event(466-518)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: unit / Ruff & mypy
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Test "not integration"
- GitHub Check: unit / Ruff & mypy
🔇 Additional comments (3)
plugins/gemini/vision_agents/plugins/gemini/gemini_llm.py (3)
184-187: Address the TODO comment.The TODO indicates uncertainty about whether
item_idandidxhandling is correct. This should be resolved before merging to ensure proper event tracking in follow-up rounds.Do you want me to help clarify the expected behavior for
item_idand index tracking across multi-hop tool calls, or should this be tracked in a separate issue?
151-153: Parent class methods verified successfully.Both
_dedup_and_execute(line 358) and_sanitize_tool_output(line 394) are defined in the parentLLMclass atagents-core/vision_agents/core/llm/llm.py. TheGeminiLLMclass correctly inherits fromLLM, so these method calls are valid.
66-66: Async client initialization pattern is correct.The pattern
Client(api_key=api_key).aiofollows google-genai best practices. The documentation explicitly recommends creating a single Client and reusingclient.aiorather than creating a separate AsyncClient per request, as this preserves connection pools and reduces latency.
Summary by CodeRabbit
New Features
Enhancements
Documentation
Chores